feat: add robyn-config as optional dependencies#1387
Open
Lehsqa wants to merge 1 commit into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDeclares robyn-config as an optional dependency, documents its CLI and install options, updates API reference layout/quoting, and adds integration tests that validate CLI availability, project scaffolding, adminpanel creation, and monitoring scaffolding. ChangesRobyn Config CLI Plugin
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant CLI as robyn-config
participant FS as Filesystem
TestRunner->>CLI: --help / create --help / add --help
CLI-->>TestRunner: exit 0, usage
TestRunner->>FS: create temp dir
TestRunner->>CLI: create scaffold (design, orm)
CLI-->>FS: generate pyproject.toml, src/, Makefile
TestRunner->>CLI: adminpanel, monitoring commands
CLI-->>FS: generate adminpanel dir, docker-compose.monitoring.yml, compose/monitoring/*
TestRunner->>FS: assert files exist and cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pyproject.toml (2)
78-88:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
robyn-configto Poetry dependencies.The PEP 621
[project.optional-dependencies]section includesrobyn-config, but the corresponding Poetry[tool.poetry.dependencies]section does not declare it as an optional dependency. This creates an inconsistency between installation methods.🔧 Proposed fix
[tool.poetry.dependencies] python = "^3.10" inquirerpy = "0.3.4" maturin = "1.7.4" watchdog = "^6.0.0" multiprocess = "^0.70.18" uvloop = { version = "0.22.1", markers = "sys_platform != 'win32' and (sys_platform != 'cygwin' and platform_python_implementation != 'PyPy')" } jinja2 = { version = "^3.1.6", optional = true } pydantic = { version = "^2.0.0", optional = true } +robyn-config = { version = "^1.0.0", optional = true } rustimport = "^1.3.4" orjson = "^3.11.5"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 78 - 88, The pyproject.toml is inconsistent: the PEP 621 [project.optional-dependencies] lists robyn-config but [tool.poetry.dependencies] does not declare it as optional; add an entry for robyn-config under [tool.poetry.dependencies] (e.g. robyn-config = { version = "<same-version-as-in-[project.optional-dependencies]>", optional = true }) so Poetry knows it is an optional dependency and the versions match between the two sections.
90-93:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUpdate Poetry extras to include
robyn-config.The PEP 621
[project.optional-dependencies]section's"all"extra includesrobyn-config(line 41), but the Poetry[tool.poetry.extras]section does not. Users installing viapip install robyn[all]will getrobyn-config, while those using Poetry with theallextra will not.🔧 Proposed fix
[tool.poetry.extras] templating = ["jinja2"] pydantic = ["pydantic"] -all = ["jinja2", "pydantic"] +robyn-config = ["robyn-config"] +all = ["jinja2", "pydantic", "robyn-config"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 90 - 93, The Poetry extras block ([tool.poetry.extras]) is missing the robyn-config package in the "all" extra; update the "all" list to include "robyn-config" and optionally add a dedicated extra key "robyn-config" = ["robyn-config"] so Poetry users get the same optional dependency set as the PEP 621 [project.optional-dependencies] configuration; edit the existing keys templating, pydantic and all to include the robyn-config entry (e.g., add "robyn-config" to the all array and add a new robyn-config extra if you want a named extra).
🧹 Nitpick comments (2)
integration_tests/test_robyn_config.py (2)
52-70: ⚖️ Poor tradeoffConsider parameterizing the
scaffolded_projectfixture.The
scaffolded_projectfixture always creates a DDD project with SQLAlchemy (line 64), which means the adminpanel and monitoring tests only validate one configuration. Consider making this fixture parameterized or creating separate fixtures for different design patterns to ensure broader test coverage.This is especially relevant since
test_create_project_scaffoldtests all design×ORM combinations (lines 72-89), but subsequent tests don't.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration_tests/test_robyn_config.py` around lines 52 - 70, The scaffolded_project fixture is hard-coded to ("ddd","sqlalchemy"); change it to be parameterized so tests run against multiple design×ORM combos: convert scaffolded_project to accept a pytest request (or use `@pytest.fixture`(params=[("ddd","sqlalchemy"), ("layered","tortoise"), ...])) and build the subprocess.run args from request.param (e.g., design, orm) when invoking "robyn-config create"; alternatively add distinct fixtures (e.g., scaffolded_project_ddd_sqlalchemy, scaffolded_project_layered_tortoise) that run subprocess.run with the desired design/orm combinations and update tests that use scaffolded_project to either use the parametrized fixture or the new named fixtures.
91-116: ⚖️ Poor tradeoffAdd adminpanel tests for MVC design pattern.
The
scaffolded_projectfixture hardcodes--design ddd(line 64), so the adminpanel tests (lines 102, 115) never validate MVC projects. Since MVC is a documented design pattern (as shown intest_create_project_scaffoldat line 72), consider parameterizing the adminpanel tests to cover both design patterns, or document why only DDD is tested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration_tests/test_robyn_config.py` around lines 91 - 116, The adminpanel tests (test_adminpanel_scaffolding and test_adminpanel_with_custom_credentials) only exercise the scaffolded_project fixture which currently hardcodes "--design ddd", so they never validate MVC projects; update the tests to cover both designs by either parameterizing these two tests with pytest.mark.parametrize over designs ["ddd","mvc"] and invoking the scaffold creation with the design parameter, or modify/extend the scaffolded_project fixture to accept a design argument (or add a separate mvc_scaffolded_project fixture) and run the same assertions for both designs; refer to scaffolded_project, test_adminpanel_scaffolding, test_adminpanel_with_custom_credentials and test_create_project_scaffold to implement the parameterization or fixture change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs_src/src/pages/documentation/en/plugins.mdx`:
- Around line 48-60: Update the CLI examples to match the actual robyn-config
signatures: for the create example include the required destination directory
argument (use the form "robyn-config create my_project destination_dir --design
ddd --orm sqlalchemy"), and ensure the other commands include the target project
directory argument as required by the implementation—i.e., change "robyn-config
add users", "robyn-config adminpanel", and "robyn-config monitoring" to include
the project directory parameter (e.g., "robyn-config add users <project_dir>",
"robyn-config adminpanel <project_dir>", "robyn-config monitoring
<project_dir>") so the examples align with the robyn-config
create/adminpanel/monitoring/add command signatures.
In `@integration_tests/test_robyn_config.py`:
- Around line 74-89: The test test_create_project_scaffold assumes the
robyn-config CLI supports a positional target directory (it calls subprocess.run
with ["robyn-config", "create", project_name, temp_dir, "--design", design,
"--orm", orm]) and expects created files directly in temp_dir; confirm whether
the CLI command handler for "create" accepts a second positional argument for
destination and whether implementation places files in destination vs
destination/project_name; if the CLI does not accept that positional arg or
places files under destination/project_name, either update the CLI handler to
accept and write into the provided directory or change the test to call the
documented signature (e.g., ["robyn-config", "create", project_name, "--design",
design, "--orm", orm]) and assert files exist under temp_dir/project_name, and
update docs_src/src/pages/documentation/en/plugins.mdx to match the chosen
behavior.
---
Outside diff comments:
In `@pyproject.toml`:
- Around line 78-88: The pyproject.toml is inconsistent: the PEP 621
[project.optional-dependencies] lists robyn-config but
[tool.poetry.dependencies] does not declare it as optional; add an entry for
robyn-config under [tool.poetry.dependencies] (e.g. robyn-config = { version =
"<same-version-as-in-[project.optional-dependencies]>", optional = true }) so
Poetry knows it is an optional dependency and the versions match between the two
sections.
- Around line 90-93: The Poetry extras block ([tool.poetry.extras]) is missing
the robyn-config package in the "all" extra; update the "all" list to include
"robyn-config" and optionally add a dedicated extra key "robyn-config" =
["robyn-config"] so Poetry users get the same optional dependency set as the PEP
621 [project.optional-dependencies] configuration; edit the existing keys
templating, pydantic and all to include the robyn-config entry (e.g., add
"robyn-config" to the all array and add a new robyn-config extra if you want a
named extra).
---
Nitpick comments:
In `@integration_tests/test_robyn_config.py`:
- Around line 52-70: The scaffolded_project fixture is hard-coded to
("ddd","sqlalchemy"); change it to be parameterized so tests run against
multiple design×ORM combos: convert scaffolded_project to accept a pytest
request (or use `@pytest.fixture`(params=[("ddd","sqlalchemy"),
("layered","tortoise"), ...])) and build the subprocess.run args from
request.param (e.g., design, orm) when invoking "robyn-config create";
alternatively add distinct fixtures (e.g., scaffolded_project_ddd_sqlalchemy,
scaffolded_project_layered_tortoise) that run subprocess.run with the desired
design/orm combinations and update tests that use scaffolded_project to either
use the parametrized fixture or the new named fixtures.
- Around line 91-116: The adminpanel tests (test_adminpanel_scaffolding and
test_adminpanel_with_custom_credentials) only exercise the scaffolded_project
fixture which currently hardcodes "--design ddd", so they never validate MVC
projects; update the tests to cover both designs by either parameterizing these
two tests with pytest.mark.parametrize over designs ["ddd","mvc"] and invoking
the scaffold creation with the design parameter, or modify/extend the
scaffolded_project fixture to accept a design argument (or add a separate
mvc_scaffolded_project fixture) and run the same assertions for both designs;
refer to scaffolded_project, test_adminpanel_scaffolding,
test_adminpanel_with_custom_credentials and test_create_project_scaffold to
implement the parameterization or fixture change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58b15882-1243-4fb2-950c-d57ce7e70451
📒 Files selected for processing (4)
docs_src/src/pages/documentation/en/api_reference/index.mdxdocs_src/src/pages/documentation/en/plugins.mdxintegration_tests/test_robyn_config.pypyproject.toml
Lehsqa
force-pushed
the
feature/robin-config-depends
branch
from
86435aa to
d9a0fab
Compare
Lehsqa
force-pushed
the
feature/robin-config-depends
branch
from
30b359d to
621be80
Compare
Member
|
Hey @Lehsqa , First of all, thank you for the PR. One question: do you plan to maintain Robyn config for long? |
Author
|
Hey @sansyrox Yep, I am planning to maintain for a long time. Have plans how to continue adding a new features until at least version 2.0 |
Member
|
Thanks for responding @Lehsqa :D One request - could you add some screenshots of the admin panel and any other relevant things in the docs(that come from robyn config?) |
Author
|
Yes, sure. I will do it this weekend |
Lehsqa
force-pushed
the
feature/robin-config-depends
branch
from
621be80 to
a0760ea
Compare
Lehsqa
force-pushed
the
feature/robin-config-depends
branch
from
a0760ea to
8feac16
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
integration_tests/test_robyn_config.py (2)
38-46: ⚡ Quick winAdd output verification for consistency.
The other help tests (
test_cli_help_exits_zeroandtest_cli_create_help) verify output content in addition to the exit code. Adding a similar assertion here would improve consistency and catch potential regressions in the CLI output.✅ Suggested enhancement
def test_cli_add_help(self): """robyn-config add --help should list available options.""" result = subprocess.run( ["robyn-config", "add", "--help"], capture_output=True, text=True, timeout=30, ) assert result.returncode == 0 + assert "add" in result.stdout.lower() or "Usage" in result.stdout🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration_tests/test_robyn_config.py` around lines 38 - 46, The test test_cli_add_help currently only asserts the exit code; update it to also verify the help output contents (like the other tests test_cli_help_exits_zero and test_cli_create_help) by asserting that result.stdout (or result.stderr if help prints there) contains expected strings such as "Usage", the "add" subcommand description, or relevant option flags (e.g., "--help" or "--name") to ensure the help text is present and stable; locate the subprocess.run call in test_cli_add_help and add the appropriate assertion(s) checking the help output.
105-116: 💤 Low valueConsider verifying custom credentials were applied.
The test passes
--usernameand--passwordbut only asserts that the adminpanel directory was created, not that the credentials were actually used in the generated configuration. Consider either:
- Verifying the credentials in the generated config file, or
- Updating the docstring to clarify that this test only validates successful creation with custom arguments
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration_tests/test_robyn_config.py` around lines 105 - 116, The test test_adminpanel_with_custom_credentials currently only checks adminpanel_dir exists; update it to assert the provided credentials were written into the generated adminpanel config by opening files under adminpanel_dir (e.g., the generated config or env file) and checking that "testadmin" and "testpass" appear (or adjust to the specific config key names if known), using scaffolded_project to locate adminpanel_dir; alternatively, if you intend to only validate creation, change the docstring to state the test only verifies directory creation rather than credential application.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration_tests/test_robyn_config.py`:
- Around line 59-70: The scaffolded_project fixture doesn't verify robyn-config
succeeded; update the subprocess.run call in the scaffolded_project fixture to
check the result (use check=True or inspect CompletedProcess.returncode) and, on
failure, fail the fixture with pytest.fail including the subprocess
stdout/stderr so the test output shows the real error; reference the
scaffolded_project fixture and the subprocess.run invocation to locate where to
add this check and the failure message.
---
Nitpick comments:
In `@integration_tests/test_robyn_config.py`:
- Around line 38-46: The test test_cli_add_help currently only asserts the exit
code; update it to also verify the help output contents (like the other tests
test_cli_help_exits_zero and test_cli_create_help) by asserting that
result.stdout (or result.stderr if help prints there) contains expected strings
such as "Usage", the "add" subcommand description, or relevant option flags
(e.g., "--help" or "--name") to ensure the help text is present and stable;
locate the subprocess.run call in test_cli_add_help and add the appropriate
assertion(s) checking the help output.
- Around line 105-116: The test test_adminpanel_with_custom_credentials
currently only checks adminpanel_dir exists; update it to assert the provided
credentials were written into the generated adminpanel config by opening files
under adminpanel_dir (e.g., the generated config or env file) and checking that
"testadmin" and "testpass" appear (or adjust to the specific config key names if
known), using scaffolded_project to locate adminpanel_dir; alternatively, if you
intend to only validate creation, change the docstring to state the test only
verifies directory creation rather than credential application.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13684d22-cd41-44c9-9bfe-65e19f0969fb
⛔ Files ignored due to path filters (7)
docs_src/public/images/robyn-config/admin/home.pngis excluded by!**/*.pngdocs_src/public/images/robyn-config/admin/login.pngis excluded by!**/*.pngdocs_src/public/images/robyn-config/admin/model_edit.pngis excluded by!**/*.pngdocs_src/public/images/robyn-config/admin/model_list.pngis excluded by!**/*.pngdocs_src/public/images/robyn-config/admin/settings.pngis excluded by!**/*.pngdocs_src/public/images/robyn-config/monitoring/logs.pngis excluded by!**/*.pngdocs_src/public/images/robyn-config/monitoring/metrics.pngis excluded by!**/*.png
📒 Files selected for processing (5)
docs_src/src/pages/documentation/en/api_reference/index.mdxdocs_src/src/pages/documentation/en/api_reference/testing.mdxdocs_src/src/pages/documentation/en/plugins.mdxintegration_tests/test_robyn_config.pypyproject.toml
✅ Files skipped from review due to trivial changes (2)
- docs_src/src/pages/documentation/en/api_reference/index.mdx
- docs_src/src/pages/documentation/en/api_reference/testing.mdx
Lehsqa
force-pushed
the
feature/robin-config-depends
branch
from
8feac16 to
3bb7732
Compare
Member
|
I really like it @Lehsqa :D . Doing a final review before I merge :D |
Author
|
@sansyrox Thanks for your review). I had downgraded minimum version to 3.10 for robyn-config. Pls, rerun benchmark verification. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds
robyn-configas an optional dependency to the Robyn framework, along with documentation and integration tests.Summary
robyn-config >= 1.0.0, < 2.0.0as an optional dependency group inpyproject.tomlrobyn-configin the"all"extras grouprobyn-configin the plugins page (docs/en/plugins.mdx) with both standalone (pip install robyn-config) and optional dependency (pip install robyn[robyn-config]) installation methodsdocs/en/api_reference/index.mdx) to list all available optional dependencies:robyn[templating],robyn[pydantic],robyn[robyn-config], androbyn[all]PR Checklist
Please ensure that:
Pre-Commit Instructions:
Summary by CodeRabbit
Documentation
Tests
Chores